Fix: document the gcloud pass-through of adk deploy cloud_run in --help - #587
Open
AmaadMartin wants to merge 3 commits into
Open
Fix: document the gcloud pass-through of adk deploy cloud_run in --help#587AmaadMartin wants to merge 3 commits into
AmaadMartin wants to merge 3 commits into
Conversation
added 3 commits
August 3, 2026 12:19
The three `adk deploy` subcommands were registered without a description,
so `adk deploy --help` printed three blank rows, and nothing in the repo
said that `adk deploy cloud_run` forwards unrecognized flags verbatim to
`gcloud run deploy` or that a handful of flags are reserved because ADK
sets them itself.
Add a description to each subcommand and an `addHelpText('after', ...)`
epilog on `cloud_run` that states the forwarding contract, the `--`
separator, worked examples, and the reserved flags. The epilog's reserved
list is derived from `prepareGCloudArguments` in
`dev/src/cli/deploy/cli_deploy_cloud_run.ts`.
No parsing behaviour changes.
`getExtraGcloudArgs` relied on commander to consume the end-of-options marker, but commander only discards it while it is still collecting operands: the first unrecognized flag switches the destination to the unknown list, after which the `--` is kept in `command.args`. So a mixed invocation such as adk deploy cloud_run ./agent --no-allow-unauthenticated -- --min-instances=2 forwarded `["--no-allow-unauthenticated", "--", "--min-instances=2"]`, and `gcloud run deploy` (which declares no trailing-remainder argument) aborted with `unrecognized arguments: --` after the bundle and containerize work had already run. Drop every bare `--` from the forwarded list. This matches the separator contract the cloud_run help epilog documents, and Python's click, which never passes the separator to `extra_gcloud_args` either.
Commander binds the first unmatched token to the declared `[agents_dir]` argument even when that token is an unknown flag the command is forwarding, so `adk deploy cloud_run --allow-unauthenticated` (and the documented separator form with the directory omitted, `adk deploy cloud_run -- --min-instances=2`) resolved the deploy source to `<cwd>/--allow-unauthenticated` and could never succeed. Forwarding the flag without also fixing the path left the invocation broken. Resolve the argument through `resolveAgentPath`, which falls back to the argument's own `process.cwd()` default when the value looks like a flag; an agent path never starts with `-`, since a relative one is spelled `./-name`. Also tighten the reserved-flag paragraph of the cloud_run help epilog: state the `--a2a_auth_token` condition as a condition, state the rejection once, and drop the rationale that already lives as a comment next to the list it describes in `cli_deploy_cloud_run.ts`.
This was referenced Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
N/A — no existing public issue.
Problem:
adk deploy --helpdocuments nothing about deploying.All three deploy subcommands are registered without
.description()(
dev/src/cli/cli.ts:cloud_runat theDEPLOY_COMMAND.command('cloud_run')call, and
agent_engine/reasoning_enginethroughregisterAgentEngineCommand), unlikeweb,api_server,createandrun.Commander therefore prints three blank rows:
adk deploy cloud_runforwards every option it does not recognize verbatim togcloud run deploy, and rejects a handful of them because ADK sets themitself (
validateGcloudExtraArgsindev/src/cli/deploy/cli_deploy_cloud_run.ts). Neither half of that contract iswritten down anywhere in the repo —
grep -r cloud_run docs/ README.md CONTRIBUTING.mdreturns nothing, so the help output is the documentationsurface, and it was silent. The only ways to discover the pass-through were to
read the source or to hit the error.
Solution: help text only — no parsing behaviour changes.
.description('Deploys an agent to Cloud Run')oncloud_run, and.description('Deploys an agent to Vertex AI Agent Engine')insideregisterAgentEngineCommand(which serves bothagent_engineandreasoning_engine), so no row inadk deploy --helpis blank..addHelpText('after', CLOUD_RUN_HELP_EPILOG)oncloud_run, printing aftercommander's option block:
CLOUD_RUN_HELP_EPILOGis a module-levelconstnext to the other CLIconstants (not exported, not inlined into the chain), matching how this file
already keeps its option constants at module scope.
Every factual claim in the epilog is derived from the code, not from memory:
gcloud run deploygetExtraGcloudArgs(cli.ts) →extraGcloudArgs→ appended to the argv inprepareGCloudArguments(cli_deploy_cloud_run.ts)--source,--project,--port,--verbosityreservedadkManagedArgsinprepareGCloudArguments--regionreserved unconditionallyadkManagedArgs.push('--region')is guarded byoptions.region, anddeployToCloudRunresolves and assignsoptions.region(from--regionorgcloud config get-value run/region, throwing if neither exists) before callingprepareGCloudArguments— so it is always set by then. Confirmed against the built CLI in all three paths, see Verifying the--regionclaim below--a2a_auth_tokenif (options.a2aAuthToken)branch inprepareGCloudArguments--separates gcloud args from adk argsgetExtraGcloudArgsnow drops every bare--, so the separator never reaches gcloud (pinned by three tests: the exact example the help prints, a mixed invocation, and a double separator)Verifying the
--regionclaim. It would be easy to readif (options.region) adkManagedArgs.push('--region')as "reserved only when theuser passes
--region". It is not:deployToCloudRunassignsoptions.regionbefore
prepareGCloudArgumentsruns, so the flag can never be passed through.Checked against the built CLI, all three paths:
The epilog therefore states
--regionunconditionally, which is what the CLIdoes. The one genuinely conditional item — the env-var flags — is written as a
condition.
Wording parity with the Python SDK. The epilog mirrors the
cli_deploy_cloud_rundocstring in adk-python (Use '--' to separate gcloud arguments from adk arguments.plus the two worked examples), extended with thereserved-flag list, which adk-python's docstring does not state but its
_validate_gcloud_extra_argsenforces.Second commit — make the documented
--separator actually work. Writing"Use -- to separate gcloud arguments from adk arguments" into the help is only
honest if the separator survives the round trip, and it did not:
The base of this stack (#447) reads the pass-through list off commander's parse
result, on the assumption that commander always consumes the terminator. It does
not:
commander/lib/command.jshandles the marker withif (arg === '--') { if (dest === unknown) dest.push(arg); … }, and the firstunrecognized flag flips
destfromoperandstounknown. So the--isdiscarded only when it is the first unrecognized token; once any loose gcloud
flag precedes it, the marker is retained in
command.argsand copied straightthrough.
validateGcloudExtraArgswaves it past (a bare--conflicts withnothing),
prepareGCloudArgumentsappends it to the argv, and — becausegcloud run deploydeclares no trailing-remainder argument — the deploy abortswith
ERROR: (gcloud.run.deploy) unrecognized arguments: --, exit 2, after thebundle and containerize work has already run.
Fix:
getExtraGcloudArgsfilters every bare--out of what it returns. This isthe whole delta — one
.filterand a comment explaining the commander behaviourthat makes it necessary. Dropping every bare marker rather than just the first
is deliberate: a second
--is equally unrecognizable to gcloud, so forwardingit would only produce the same exit-2 failure. Python's click likewise never
passes the separator into
extra_gcloud_args(
test_cli_deploy_cloud_run_allows_empty_gcloud_args).Third commit — don't deploy a leading gcloud flag as the agent directory.
Commander binds the first unmatched token to the declared
[agents_dir]argument even when that token is an unknown flag being forwarded. So with the
directory omitted:
The base of this stack fixed the forwarding half of that case (the flag now
reaches gcloud instead of being silently swallowed) but left the path half
broken, so the invocation still could not succeed. Resolving the argument
through
resolveAgentPath— fall back to the argument's ownprocess.cwd()default when the value looks like a flag — completes it. An agent path never
starts with
-; a relative one is spelled./-name.Not breaking:
.description()and.addHelpText()cannot affect parsing; theseparator filter only changes invocations that use
--, every one of whicheither already worked (separator first — unchanged) or failed with the
unrecognized-argument error above; and
resolveAgentPathonly changes an agentpath that begins with
-, which never resolved to an existing directory. Thereis no working behaviour to preserve in either case. No new exported symbol, no
dependency, no lockfile change, no
.mdfile touched.Testing Plan
Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.
Commands run (targeted only):
Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.
Seven new cases in
dev/test/cli/cli_test.ts, all added — no existing test wasedited, renamed, skipped or deleted:
command: deploy > should list every deploy subcommand with a description—asserts each of the three rows carries its description.
command: deploy cloud_run > should document the gcloud pass-through contract in its help— asserts the description, the forwarding sentence, the--sentence, the worked
--example, the reserved-flag sentence, the env-varsentence, that the epilog comes after the option block, and that
deployToCloudRunis never called by a help request.command: deploy cloud_run > should forward the -- example printed in its help— runs the literal example from the epilog(
path/to/my_agent -- --min-instances=2) and assertsextraGcloudArgs === ['--min-instances=2'], tying the documentation to thebehaviour.
command: deploy cloud_run > should drop the -- separator when a gcloud flag precedes it— the mixed invocation that was broken:./my-agent-path --no-allow-unauthenticated -- --min-instances=2must forward['--no-allow-unauthenticated', '--min-instances=2'].command: deploy cloud_run > should drop every bare -- from the forwarded gcloud args— a space-separated unknown flag plus two separators(
--memory 512Mi -- --min-instances=2 -- --cpu=2) must forward['--memory', '512Mi', '--min-instances=2', '--cpu=2'], pinning that nomarker survives in any position.
command: deploy cloud_run > should not deploy a leading gcloud flag as the agent directory—deploy cloud_run --allow-unauthenticatedmust resolveagentPathto the working directory, not to<cwd>/--allow-unauthenticated.Paired with the existing forwarding assertion for the same invocation rather
than editing it.
command: deploy cloud_run > should support the -- example with the agent directory omitted—deploy cloud_run -- --min-instances=2must both resolveagentPathto the working directory and forward['--min-instances=2'],i.e. the form this help documents works with or without the directory.
Tests 1-3 share one module-level helper,
captureHelp(program, commandPath), whichdrives the real
--helppath (program.parseAsync([... , '--help'])) ratherthan calling a formatting method directly, and asserts the run exits with code
0/commander.helpDisplayed. Two commander details it accounts for, bothverified against
commander@14.0.3(the version the lockfile resolves fordev's"commander": "^14.0.0"):helpInformation()does not includeaddHelpTextoutput — the epilog isemitted by
outputHelp()via theafterHelpevent. Asserting onhelpInformation()would silently miss it.configureOutput()assigns a new_outputConfigurationobject on thecommand it is called on, while subcommands captured the parent's object at
creation time. Capturing on the root program therefore does not redirect a
subcommand's help, so the helper configures (and
exitOverrides) the commandthat renders the help.
Proof each new test can fail (mutations applied one at a time to the
already-passing tree, then reverted):
.description('Deploys an agent to Cloud Run')expected 'Usage: ADK CLI deploy [options] [comm…' to contain 'cloud_run [options] [agents_dir] Depl…'andexpected 'Usage: ADK CLI deploy cloud_run [opti…' to contain 'Deploys an agent to Cloud Run'.addHelpText('after', CLOUD_RUN_HELP_EPILOG)expected 'Usage: ADK CLI deploy cloud_run [opti…' to contain 'Any option that is not listed above i…'(1 failed, 34 passed).description('Deploys an agent to Vertex AI Agent Engine')expected 'Usage: ADK CLI deploy [options] [comm…' to contain 'agent_engine [options] [agents_dir] D…'(1 failed, 34 passed)shift()ingetExtraGcloudArgs(base behaviour this PR documents)expected [ 'path/to/my_agent', …(1) ] to deeply equal [ '--min-instances=2' ]return extraArgs;instead ofreturn extraArgs.filter(…)expected [ '--no-allow-unauthenticated', …(2) ] to deeply equal [ '--no-allow-unauthenticated', …(1) ]andexpected [ '--memory', '512Mi', '--', …(3) ] to deeply equal [ '--memory', '512Mi', …(2) ](2 failed, 35 passed)agentPath: getAbsolutePath(agentPath)instead ofresolveAgentPath(agentPath)expected '/…/--allow-unauthenticated' to be '/…' // Object.is equalityandexpected { …(16) } to match object { …(2) }(2 failed, 37 passed)Tests 4-7 were each written first and observed failing against the unfixed tree,
with exactly the output above, before the corresponding fix was applied.
Coverage. Every new source line is executed — statement hits for the new
lines, from
--coverage.include='dev/src/cli/cli.ts':[[118,17],[119,17],[130,17],[131,17],[231,1],[449,39],[478,39],[489,17],[514,78]].Both new branches have both outcomes covered: the
.filterpredicate(
[118, [21]]) and theresolveAgentPathternary ([131, [3]]/[131, [14]]— flag-shaped values and normal paths). Whole-file branch coverage moves up,
72.5% → 75%. The whole-file line number when running only this file is 95.41%;
every uncovered line (51-52, 95-96, 303-305, 348-350, 387-388, 438-439, 507-508,
562-563, 590-594) is pre-existing error-handling in other commands, none of them
in this diff.
Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.
Run against the real built CLI, no mocks (
npm run build -w dev, thennode dev/dist/esm/cli_entrypoint.js …):adk deploy --help→ exit 0, and the three rows now read:adk deploy cloud_run --help→ exit 0, epilog printed immediately after the-h, --helprow, exactly as quoted above.Reserved-flag claim, verified live (no network, no deploy — validation fails
first), in both the plain and the previously-broken mixed form:
Both forms carry the post-separator token through to
validateGcloudExtraArgs, i.e. the epilog's claim holds whether or not aloose gcloud flag precedes the separator.
Same check with the agent directory omitted — previously this could not get
as far as validation, because
--port=9999was bound to[agents_dir]:Not run: an actual
adk deploy cloud_run … -- --min-instances=2against alive GCP project, which needs credentials and a billable deploy. The absence
of the
--in the forwarded argv is not observable from outside the process—
validateGcloudExtraArgsignores a bare marker and onlygclouditselfrejects it — so it is asserted at the collector seam instead, by tests 3-5
above.
Checklist
[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.
Collision check.
gh pr list --repo AmaadMartin/adk-js --state open --limit 1000(full list, not truncated), filtered for deploy/cloud-run/help-text work,then
gh pr diff --name-onlyon every plausibly adjacent PR:fix/cloud-run-passthrough-gcloud-args— real overlap. Rewrites thesame pass-through collector in
dev/src/cli/cli.ts, so this branch is stackedon top of it and targets its branch rather than shipping a competing
implementation of that rewrite. It does not touch help text (its body notes
adk deploy cloud_run --helpis byte-identical). It intended to also fix the--forwarding this task asks for, but only got the leading-separator case;the second commit here completes it, as a one-line change to the function that
PR introduced.
fix/cli-bundle-file-type-help-textand Fix: enumerate all supported session and artifact service URI schemes in adk --help #385fix/cli-service-uri-help-text-schemestouch the same two files but describeoptions (
--file_type, the service-URI schemes), not subcommands, and addno epilog. Textually adjacent, no functional overlap — not stacked on.
the deploy implementation (Dockerfile pinning, exit codes, app names, env
vars) and none change help text.
Because this PR targets
fix/cloud-run-passthrough-gcloud-argsrather thanmain,.github/workflows/validation.yaml(pull_request: branches: [main])does not trigger, so no test job runs on it. Validation was done locally on the
pushed commit as recorded above.